Conversation
- Redesign /me and /user/[id] pages with modern hero sections - Fix avatar clipping by moving avatars outside overflow containers - Use Bootstrap utility classes for responsive styling - Adjust activity/create editor to responsive column layout - Fix hydration error on /me page by using SSR props directly
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR modernizes the user profile page UI, enhances session management with async cleanup and secure cookies, updates activity creation page layout, implements a profile redirect middleware, adds GitHub OAuth configuration documentation, and consolidates branding strings and configuration exports across the application. ChangesAuthentication System Enhancement
User Profile Page Modernization
Activity Creation Page Layout
Branding & Localization
Infrastructure Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/api/core.ts (1)
84-89: Harden JWT cookie with HttpOnly/Secure/SameSite flagsThe JWT session token is currently set without any security flags:
res.setHeader('Set-Cookie', `JWT=${user.token}; Path=/`);This makes the token readable from JavaScript and more exposed to XSS and CSRF‑style attacks. Add HttpOnly, Secure (in production), and SameSite flags:
- res.setHeader('Set-Cookie', `JWT=${user.token}; Path=/`); + const isProd = process.env.NODE_ENV === 'production'; + res.setHeader( + 'Set-Cookie', + [ + `JWT=${user.token}`, + 'Path=/', + 'HttpOnly', + isProd ? 'Secure' : '', + 'SameSite=Lax', + ] + .filter(Boolean) + .join('; '), + );This significantly reduces the blast radius if any XSS occurs and improves CSRF resistance for JWT‑backed requests.
🧹 Nitpick comments (4)
pages/login.tsx (1)
17-24: Consider hydration error prevention and error handling.The redirect logic looks correct, but consider these improvements:
Effect cleanup: If the component unmounts during redirect (e.g., user navigates away), the
router.replacecould throw. Consider adding cleanup or error handling.Session storage timing: Direct assignment on Line 19 (
sessionStore.user = jwtPayload) happens before the redirect completes. If the target page immediately reads session state, ensure MobX reactivity has settled.No-op when jwtPayload is absent: The effect only runs when
jwtPayloadexists, which is correct. However, consider what happens if a user reaches this page without a valid session—should there be a fallback UI or redirect to home?Consider adding error boundary or try-catch:
useEffect(() => { if (jwtPayload) { sessionStore.user = jwtPayload; const targetUrl = typeof redirect === 'string' ? redirect : '/'; - router.replace(targetUrl); + router.replace(targetUrl).catch(error => { + console.error('Redirect failed:', error); + }); } }, [jwtPayload, redirect, router]);pages/me/index.tsx (1)
48-68: Optional: consider factoring repeated avatar/action patterns into shared componentsThe avatar block and some of the action button patterns overlap conceptually with
/user/[id].tsx. Extracting a smallAvatarand/orProfileActionscomponent could reduce duplication and keep behavior (e.g., future avatar sizing or styling tweaks) consistent across pages.Also applies to: 166-188
pages/user/[id].tsx (2)
75-89: GitHub link assumesnameis a username; consider using an explicit field or guardingThe GitHub button currently links to:
href={`https://github.com/${name}`}This only works reliably if
nameis guaranteed to be the GitHub handle. In many user models,nameis a display name, which can contain spaces or non‑ASCII characters and won’t map cleanly to a GitHub profile URL.Consider either:
- Using an explicit
githubLogin(or similar) field fromUser, if available, or- Guarding the link so it’s only active when you have a valid GitHub handle, e.g.:
- <a - href={`https://github.com/${name}`} + {name && ( + <a + href={`https://github.com/${encodeURIComponent(name)}`} target="_blank" rel="noreferrer" ... - > - <Icon name="github" /> - </a> + > + <Icon name="github" /> + </a> + )}This avoids obviously broken or misleading links for users whose
nameisn’t their GitHub username.
90-120: Optional: clarify placeholder social buttons to avoid implying wired accountsThe QQ/WeChat/Weibo icons are rendered as styled
<span>elements without any actual linking or behavior. Visually they look like active buttons, which can confuse users if they click and nothing happens.If these are placeholders for future work, consider:
- Reducing opacity / disabling pointer events, or
- Hiding them until real profile data (e.g.,
That keeps the UI honest about which socials are actually connected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.env.development(1 hunks)README.md(1 hunks)components/User/UserBar.tsx(2 hunks)docs/TASKS.md(1 hunks)models/User/Session.ts(1 hunks)pages/_app.tsx(2 hunks)pages/activity/create.module.less(0 hunks)pages/activity/create.tsx(1 hunks)pages/api/core.ts(1 hunks)pages/login.tsx(1 hunks)pages/me/index.module.less(1 hunks)pages/me/index.tsx(1 hunks)pages/user/[id].module.less(1 hunks)pages/user/[id].tsx(2 hunks)
💤 Files with no reviewable changes (1)
- pages/activity/create.module.less
🧰 Additional context used
🧬 Code graph analysis (5)
pages/me/index.tsx (5)
pages/activity/create.tsx (1)
getServerSideProps(14-14)pages/login.tsx (1)
getServerSideProps(11-11)pages/user/[id].tsx (1)
getServerSideProps(19-24)pages/api/core.ts (1)
sessionGuard(115-115)models/Base/Translation.ts (1)
I18nContext(44-44)
pages/login.tsx (4)
pages/activity/create.tsx (1)
getServerSideProps(14-14)pages/me/index.tsx (1)
getServerSideProps(15-15)pages/api/core.ts (1)
sessionGuard(115-115)components/layout/PageHead.tsx (1)
PageHead(11-19)
components/User/UserBar.tsx (1)
components/layout/MainNavigation.tsx (1)
() => import('../User/UserBar')(10-10)
models/User/Session.ts (1)
configuration/index.ts (3)
API_HOST(10-10)isServer(3-3)globalThis(12-15)
pages/api/core.ts (3)
configuration/index.ts (1)
process(8-8)models/Git.ts (1)
next(15-21)next.config.ts (1)
beforeFiles(25-38)
🪛 dotenv-linter (4.0.0)
.env.development
[warning] 4-4: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
🔇 Additional comments (8)
README.md (1)
29-57: LGTM! Clear and comprehensive setup documentation.The environment configuration section provides excellent guidance for developers. The OAuth setup instructions are particularly helpful.
pages/activity/create.tsx (1)
33-45: LGTM! Well-structured responsive layout.The Bootstrap grid implementation is clean and follows best practices. The responsive column sizing (lg=10, xl=8) provides appropriate width constraints for the form.
docs/TASKS.md (1)
1-191: LGTM! Excellent project planning documentation.The task breakdown is comprehensive and well-structured, with clear acceptance criteria and scope definitions for each milestone. This will help maintain organized development flow.
pages/me/index.module.less (1)
1-18: LGTM! Clean styling module.The styles are well-structured with appropriate transitions and theming. The gradient and hover effects enhance the UI.
pages/user/[id].module.less (1)
1-46: LGTM! Consistent styling with good interaction states.The styling module provides good visual feedback with transitions and hover states. The use of
:globalselectors for Bootstrap classes is appropriate.pages/me/index.tsx (1)
17-27: Session source and hydration behavior look soundUsing
jwtPayloadfromgetServerSidePropsas the primary user source and mirroring it intosessionStore.userinuseEffect, while resolvinguser = jwtPayload || sessionStore.user, avoids hydration mismatches and still keeps the MobX store in sync. The null fallback whenuseris missing is a reasonable guard givensessionGuardshould already handle redirects.components/User/UserBar.tsx (1)
14-18: Login redirect handling is correct locally; verify server-side validation
loginUrlcorrectly encodesrouter.asPath, and the UI cleanly distinguishes authenticated (dropdown) vs unauthenticated (GitHub sign‑in button) states. The only remaining concern is how/loginconsumes theredirectquery:
- If
/loginblindly redirects toredirect, a crafted URL could potentially be used for open redirects.- Ideally, the login handler should allow only internal paths and ignore or sanitize absolute URLs.
Given that logic lives outside this file, please confirm
/pages/login.tsxvalidatesredirectas an internal path before using it.Also applies to: 25-49
pages/_app.tsx (1)
14-16: > Likely an incorrect or invalid review comment.
- fix(.env.development): remove spaces around = in env var assignments - fix(api/core): harden JWT cookie with HttpOnly, SameSite=Lax, Secure(prod) - fix(Session): suppress redundant error re-throw after 401/403 redirect - fix(login): add .catch() on router.replace() and guard against open redirect - fix(user/[id]): guard GitHub link with conditional render + encodeURIComponent - fix(user/[id]): remove placeholder social buttons with no backing data
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
models/User/Session.ts (1)
22-28:⚠️ Potential issue | 🟡 MinorConsider redirecting only on 401, not 403.
Per the project maintainer's feedback, 401 indicates the user is unauthenticated (JWT invalid/expired), while 403 means the resource is forbidden for the current user. Redirecting to login on 403 is inappropriate since re-authenticating won't grant access to a forbidden resource.
- if (status === 401 || status === 403) { + if (status === 401) {Additionally, ESLint flags a missing blank line before the
returnstatement on line 28.globalThis.location?.assign(loginUrl); + return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@models/User/Session.ts` around lines 22 - 28, Change the status check so only a 401 triggers a client-side redirect (i.e., replace the combined check `if (status === 401 || status === 403)` with `if (status === 401)`), leaving 403 handling untouched; keep the existing logic that builds `currentPath`, `loginUrl`, and calls `globalThis.location?.assign(loginUrl)`. Also add the missing blank line before the `return` statement inside this block to satisfy the ESLint rule. Locate this change in the session/auth handling block where `status`, `isServer()`, `currentPath`, `loginUrl`, and `globalThis.location?.assign` are used.
🧹 Nitpick comments (1)
pages/login.tsx (1)
1-9: Fix import order per ESLint.ESLint flags the imports need sorting. Run autofix to resolve.
import { User } from '@freecodecamp-chengdu/hop-service'; import { JWTProps } from 'next-ssr-middleware'; import { useRouter } from 'next/router'; import { FC, useEffect } from 'react'; import { Container, Spinner } from 'react-bootstrap'; import { PageHead } from '../components/layout/PageHead'; +import { sessionGuard } from './api/core'; import sessionStore from '../models/User/Session'; -import { sessionGuard } from './api/core';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/login.tsx` around lines 1 - 9, Reorder the import statements in this module to satisfy ESLint import-sorting: group and sort them (builtin/third-party first alphabetically, then local project imports) so that imports like "react" hooks (useEffect, FC), "next" packages (useRouter), "@freecodecamp-chengdu/hop-service" (User), "next-ssr-middleware" (JWTProps), "react-bootstrap" (Container, Spinner) appear before project-local imports (PageHead, sessionStore, sessionGuard) and ensure each import line is alphabetized within its group; you can run the project's ESLint autofix or manually reorder the imports to match the linter's expected order (refer to the imported symbols User, JWTProps, useRouter, FC, useEffect, Container, Spinner, PageHead, sessionStore, sessionGuard).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/user/`[id].tsx:
- Around line 26-39: UserDetailPage currently renders the raw email prop
directly (props: id, name, avatar, email) in two places; change it to
conditionally display the email only when the viewer is authorized (e.g., viewer
is the profile owner or the user has opted into public email) or else show a
masked/stubbed string. Update the UserDetailPage component to (1) obtain viewer
context (e.g., currentUser from AuthContext or similar), (2) implement a small
helper check like isOwnerOrConsented = currentUser?.id === id ||
user?.showEmailConsent, and (3) replace direct uses of {email} at both render
sites with a conditional that either shows the real email (or a mailto link)
when isOwnerOrConsented is true or a masked placeholder (e.g., "email hidden")
otherwise.
- Around line 77-90: The anchor rendering the icon-only social link (the <a>
around <Icon name="github" />) lacks an explicit accessible name for screen
readers; add an aria-label (or aria-labelledby) to that anchor—e.g., "GitHub" or
a more descriptive "GitHub profile for {name}"—so assistive tech has a reliable
name in addition to the visual icon and existing title attribute; update the
anchor element where classNames/styles and title="GitHub" are set to include the
aria attribute.
- Around line 76-79: The UI is linking to GitHub using User.name which is a
display name, not a guaranteed GitHub login; update the data shape and usages to
use a dedicated githubLogin (or github_login) field on the User type and replace
occurrences of user.name (including the link in pages/user/[id].tsx and the
addCollaborator(...) call) with user.githubLogin; if you cannot change the
upstream type immediately, add a validation/fetch step where the page loads
(e.g., in the user data loader or useEffect that fetches user details) to
resolve/verify the actual GitHub username via your user service or GitHub API
and only render the anchor or call addCollaborator when a validated githubLogin
exists, falling back to hiding the link and preventing addCollaborator from
using display name.
---
Duplicate comments:
In `@models/User/Session.ts`:
- Around line 22-28: Change the status check so only a 401 triggers a
client-side redirect (i.e., replace the combined check `if (status === 401 ||
status === 403)` with `if (status === 401)`), leaving 403 handling untouched;
keep the existing logic that builds `currentPath`, `loginUrl`, and calls
`globalThis.location?.assign(loginUrl)`. Also add the missing blank line before
the `return` statement inside this block to satisfy the ESLint rule. Locate this
change in the session/auth handling block where `status`, `isServer()`,
`currentPath`, `loginUrl`, and `globalThis.location?.assign` are used.
---
Nitpick comments:
In `@pages/login.tsx`:
- Around line 1-9: Reorder the import statements in this module to satisfy
ESLint import-sorting: group and sort them (builtin/third-party first
alphabetically, then local project imports) so that imports like "react" hooks
(useEffect, FC), "next" packages (useRouter),
"@freecodecamp-chengdu/hop-service" (User), "next-ssr-middleware" (JWTProps),
"react-bootstrap" (Container, Spinner) appear before project-local imports
(PageHead, sessionStore, sessionGuard) and ensure each import line is
alphabetized within its group; you can run the project's ESLint autofix or
manually reorder the imports to match the linter's expected order (refer to the
imported symbols User, JWTProps, useRouter, FC, useEffect, Container, Spinner,
PageHead, sessionStore, sessionGuard).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.developmentmodels/User/Session.tspages/api/core.tspages/login.tsxpages/user/[id].tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- .env.development
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/user/[id].tsx (1)
20-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
mobilePhone) are still exposed in__NEXT_DATA__despite the client-side guard.
getServerSidePropsreturns the fullUserobject. Next.js serializes all returned props into<script id="__NEXT_DATA__">in the HTML, which is visible to anyone who views page source or inspects the network response — regardless of whether the fields are rendered on screen. ThevisibleEmail/isOwnerguard introduced in the component only hides the display; it does not prevent the data from being embedded in the page payload. Withcache()active, this cached payload is served to every visitor.Explicitly omit sensitive fields before returning from
getServerSideProps:🔒 Proposed fix
async ({ params: { id = '' } = {} }) => { - JSON.parse(JSON.stringify({ props: await userStore.getOne(id) })), + const { email: _email, mobilePhone: _phone, password: _pw, ...safeUser } = + await userStore.getOne(id); + return JSON.parse(JSON.stringify({ props: safeUser })); },With sensitive fields removed from SSR props, update the component to source
sessionStore.user(already done formobilePhone):-const UserDetailPage: FC<User> = observer(({ id, name, avatar, email }) => { +const UserDetailPage: FC<Omit<User, 'email' | 'mobilePhone' | 'password'>> = observer(({ id, name, avatar }) => { const { t } = useContext(I18nContext); const isOwner = sessionStore.user?.id === id; - const visibleEmail = isOwner ? sessionStore.user?.email || email : undefined; + const visibleEmail = isOwner ? sessionStore.user?.email : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/user/`[id].tsx around lines 20 - 25, getServerSideProps is returning the full User from userStore.getOne(id) which embeds sensitive fields (email, mobilePhone) into the serialized __NEXT_DATA__; fetch the user via userStore.getOne(id) inside the compose async handler, construct a sanitized object that omits email and mobilePhone (or explicitly picks only public fields), and return JSON.stringify/JSON.parse of { props: sanitizedUser } instead of the raw User; keep compose, cache(), and errorLogger intact and ensure the page component sources email only from sessionStore.user while mobilePhone is also excluded from SSR props.
🧹 Nitpick comments (2)
pages/user/[id].tsx (2)
112-145: ⚡ Quick winAll three
Tab.Panes (and theirActivityListAPI calls) mount eagerly on page load.React-Bootstrap renders all
Tab.Panechildren immediately by default, even non-active ones. This triggers threeActivityListdata fetches on mount instead of one. AddingmountOnEntertoTab.Containerdefers the two inactive panes until they are first visited:⚡ Proposed fix
-<Tab.Container defaultActiveKey="enrolled"> +<Tab.Container defaultActiveKey="enrolled" mountOnEnter>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/user/`[id].tsx around lines 112 - 145, Tab panes (Tab.Pane) are all mounting eagerly causing three ActivityList API calls on page load; update the Tab.Container component (where Tab.Container is declared) to add the mountOnEnter prop so inactive Tab.Pane children defer mounting until first visited, ensuring ActivityList (type="enrolled"/"created"/"admin") only fetches when its pane is entered.
1-14: ⚡ Quick winFix ESLint import sort error.
The static analysis tool reports an error on these imports (
simple-import-sort/imports). Runeslint --fixto auto-sort them and unblock CI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/user/`[id].tsx around lines 1 - 14, The import statements in pages/user/[id].tsx are out of order and trigger the simple-import-sort/imports ESLint rule; run eslint --fix or manually reorder imports so builtin/react imports come first, then external packages (e.g., classNames, idea-react, mobx-react, next, next-ssr-middleware), then local modules (e.g., PageHead, I18nContext, sessionStore, userStore, styles), ensuring grouped and alphabetized order; after reordering, re-run lint to confirm the error for the module imports (Icon, observer, dynamic, cache/compose/errorLogger, FC/useContext, Badge/Button/Card/etc.) is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/user/`[id].tsx:
- Around line 27-30: The current isOwner calculation in UserDetailPage (using
sessionStore.user) causes a hydration mismatch because sessionStore is empty
during SSR; instead defer ownership detection until after hydration by
introducing client-only state: create a useState flag (e.g., isOwner)
initialized to false and set it inside a useEffect that compares
sessionStore.user?.id with the page prop id, then use that flag for visibleEmail
and owner-only UI. Update references to isOwner and visibleEmail so they rely on
the post-hydration state (and keep the SSR-rendered HTML unaffected).
---
Outside diff comments:
In `@pages/user/`[id].tsx:
- Around line 20-25: getServerSideProps is returning the full User from
userStore.getOne(id) which embeds sensitive fields (email, mobilePhone) into the
serialized __NEXT_DATA__; fetch the user via userStore.getOne(id) inside the
compose async handler, construct a sanitized object that omits email and
mobilePhone (or explicitly picks only public fields), and return
JSON.stringify/JSON.parse of { props: sanitizedUser } instead of the raw User;
keep compose, cache(), and errorLogger intact and ensure the page component
sources email only from sessionStore.user while mobilePhone is also excluded
from SSR props.
---
Nitpick comments:
In `@pages/user/`[id].tsx:
- Around line 112-145: Tab panes (Tab.Pane) are all mounting eagerly causing
three ActivityList API calls on page load; update the Tab.Container component
(where Tab.Container is declared) to add the mountOnEnter prop so inactive
Tab.Pane children defer mounting until first visited, ensuring ActivityList
(type="enrolled"/"created"/"admin") only fetches when its pane is entered.
- Around line 1-14: The import statements in pages/user/[id].tsx are out of
order and trigger the simple-import-sort/imports ESLint rule; run eslint --fix
or manually reorder imports so builtin/react imports come first, then external
packages (e.g., classNames, idea-react, mobx-react, next, next-ssr-middleware),
then local modules (e.g., PageHead, I18nContext, sessionStore, userStore,
styles), ensuring grouped and alphabetized order; after reordering, re-run lint
to confirm the error for the module imports (Icon, observer, dynamic,
cache/compose/errorLogger, FC/useContext, Badge/Button/Card/etc.) is resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de905dea-3310-459d-ab31-346736ef890b
📒 Files selected for processing (11)
.env.env.developmentREADME.mdcomponents/User/UserBar.tsxconfiguration/index.tsmodels/User/Session.tspages/api/core.tspages/user/[id].tsxpublic/manifest.jsontranslation/zh-CN.tstranslation/zh-TW.ts
✅ Files skipped from review due to trivial changes (7)
- .env.development
- public/manifest.json
- README.md
- configuration/index.ts
- translation/zh-TW.ts
- models/User/Session.ts
- translation/zh-CN.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/api/core.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/manifest.json`:
- Around line 2-3: The site name is inconsistent: manifest.json sets "name":
"黑客松开放平台" and "short_name": "开源黑客松" while the runtime env var
NEXT_PUBLIC_SITE_NAME is "黑客松开放"; update the canonical name to be consistent
across both places — choose the preferred product name and then update the
manifest keys ("name" and/or "short_name") and the environment variable
NEXT_PUBLIC_SITE_NAME to match that exact string so the manifest and runtime use
the same branding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c64150b8-809c-4e53-82b8-4a8bf7dc1064
📒 Files selected for processing (10)
README.mdcomponents/User/UserBar.tsxconfiguration/index.tspages/api/core.tspages/user/[id].module.lesspages/user/[id].tsxpublic/manifest.jsontranslation/en-US.tstranslation/zh-CN.tstranslation/zh-TW.ts
✅ Files skipped from review due to trivial changes (4)
- translation/en-US.ts
- translation/zh-CN.ts
- translation/zh-TW.ts
- README.md
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
pages/user/[id].tsx (1)
114-121: Handle the asyncsignOut()promise explicitly on Line 120.
sessionStore.signOut()is async and callsclear()from idb-keyval without internal error handling. If the IndexedDB operation fails, the returned promise will reject, causing an unhandled rejection. This pattern occurs at all three call sites (pages/user/[id].tsx:120, components/User/UserBar.tsx:30, and components/User/UserBar.tsx:35).♻️ Minimal safe tweak
- onClick={() => sessionStore.signOut()} + onClick={() => { + void sessionStore.signOut().catch((error) => { + console.error('Sign out failed:', error); + }); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/user/`[id].tsx around lines 114 - 121, The onClick currently calls the async sessionStore.signOut() without awaiting or handling rejections; update each call site (sessionStore.signOut() in pages/user/[id].tsx and both occurrences in components/User/UserBar.tsx) to explicitly handle the returned promise — either make the onClick handler async and await sessionStore.signOut() inside a try/catch that logs or handles errors, or append .catch(...) to the promise to swallow/log failures; ensure you preserve existing behavior (e.g., keep href/target) while preventing unhandled promise rejections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env:
- Line 1: The environment variable assignment for NEXT_PUBLIC_SITE_NAME
currently has spaces around the equals sign; update the .env entry for
NEXT_PUBLIC_SITE_NAME to use a direct KEY=value format (no spaces around '=') so
dotenv/shell parsers and linters accept it and the variable is read correctly.
In `@models/User/Session.ts`:
- Around line 61-69: The signOut function can throw if clear() (IndexedDB
cleanup) rejects, causing unhandled rejections and preventing the subsequent
reload; update async signOut(reload = false) to catch and swallow errors from
clear() (e.g., try { await clear(); } catch (err) { /* log/warn but continue */
}) so failures in clear() do not block this.user reset or the optional
location.reload(), and add a minimal debug log inside the catch to aid
diagnosis; ensure callers in pages/user/[id].tsx and components/User/UserBar.tsx
can continue to call signOut() without surrounding error handling.
In `@pages/_app.tsx`:
- Line 50: The call sessionStore.getProfile().catch(console.debug) uses
console.debug which violates the no-console rule; replace it with an allowed
logger or remove the catch handler. Update the catch to use an approved method
(e.g., sessionStore.getProfile().catch(err => logger.warn('getProfile failed',
err)) or .catch(err => console.info(err)) if a shared logger isn't available),
or remove the .catch entirely if upstream handles errors; locate the occurrence
by searching for sessionStore.getProfile() in the file and change the catch
argument from console.debug to an allowed logger method (warn/info/error) or a
no-op.
In `@pages/user/`[id].module.less:
- Line 33: Stylelint flags the CSS Modules :global(...) pseudo-class as unknown
for selectors like :global(.nav-link) (and the other :global(...) at the later
selector). Add an inline stylelint disable comment immediately above each
offending selector, e.g. /* stylelint-disable-next-line
selector-pseudo-class-no-unknown */ so stylelint ignores the :global
pseudo-class for those lines, or alternatively update project stylelint config
to allow the :global pseudo-class if you prefer a global fix; target the
specific selectors :global(.nav-link) and the other :global(...) occurrences
when applying the change.
In `@pages/user/me.tsx`:
- Line 24: Add a blank line before the existing "return result;" statement to
satisfy the `@stylistic/padding-line-between-statements` rule; locate the return
of the variable "result" in the function or component that contains "return
result;" and insert one empty line immediately above it.
---
Nitpick comments:
In `@pages/user/`[id].tsx:
- Around line 114-121: The onClick currently calls the async
sessionStore.signOut() without awaiting or handling rejections; update each call
site (sessionStore.signOut() in pages/user/[id].tsx and both occurrences in
components/User/UserBar.tsx) to explicitly handle the returned promise — either
make the onClick handler async and await sessionStore.signOut() inside a
try/catch that logs or handles errors, or append .catch(...) to the promise to
swallow/log failures; ensure you preserve existing behavior (e.g., keep
href/target) while preventing unhandled promise rejections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c451233-02c6-4635-80c3-3374b6082be9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.envcomponents/User/UserBar.tsxmodels/Activity/Award.tsmodels/User/Session.tspackage.jsonpages/_app.tsxpages/api/core.tspages/user/[id].module.lesspages/user/[id].tsxpages/user/me.tsxtsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/api/core.ts
| @@ -1,4 +1,4 @@ | |||
| NEXT_PUBLIC_SITE_NAME = 开放黑客松 | |||
| NEXT_PUBLIC_SITE_NAME = 黑客松开放平台 | |||
There was a problem hiding this comment.
Remove spaces around = in env assignment.
Line 1 uses KEY = value; prefer KEY=value to avoid parser/shell compatibility issues and satisfy dotenv linting.
Proposed fix
-NEXT_PUBLIC_SITE_NAME = 黑客松开放平台
+NEXT_PUBLIC_SITE_NAME=黑客松开放平台📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NEXT_PUBLIC_SITE_NAME = 黑客松开放平台 | |
| NEXT_PUBLIC_SITE_NAME=黑客松开放平台 |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 1-1: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env at line 1, The environment variable assignment for
NEXT_PUBLIC_SITE_NAME currently has spaces around the equals sign; update the
.env entry for NEXT_PUBLIC_SITE_NAME to use a direct KEY=value format (no spaces
around '=') so dotenv/shell parsers and linters accept it and the variable is
read correctly.
| async signOut(reload = false) { | ||
| setCookie('token', '', { path: '/', expires: new Date() }); | ||
| setCookie('JWT', '', { path: '/', expires: new Date() }); | ||
|
|
||
| this.user = undefined; | ||
| await sleep(); | ||
| await clear(); | ||
|
|
||
| if (reload) location.reload(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In idb-keyval v6.x, can clear() reject when IndexedDB is unavailable or blocked (e.g., private mode / storage restrictions)?
💡 Result:
Yes. In idb-keyval v6.x, clear returns a Promise that rejects if the underlying IndexedDB request/transaction fails (which includes cases where IndexedDB can’t be opened/used due to privacy/storage restrictions).
Citations:
- 1: https://raw.githubusercontent.com/jakearchibald/idb-keyval/3edd130da83f4eab2b049921955ae5e122889a9d/dist/idb-keyval.mjs
- 2: https://developer.mozilla.org/en-US/docs/Web/API/IDBObjectStore/clear
- 3: http://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB
🏁 Script executed:
# First, locate and read the Session.ts file around the target lines
fd -t f "Session.ts" -x wc -l {}Repository: FreeCodeCamp-Chengdu/HOP
Length of output: 96
🏁 Script executed:
# Read the file content around lines 61-69
fd -t f "Session.ts" -x head -75 {} | tail -20Repository: FreeCodeCamp-Chengdu/HOP
Length of output: 518
🏁 Script executed:
# Also check the imports and structure
fd -t f "Session.ts" -x head -10 {}Repository: FreeCodeCamp-Chengdu/HOP
Length of output: 542
🏁 Script executed:
# Search for signOut call sites
rg "signOut\(" --type ts --type tsx -B 2 -A 2Repository: FreeCodeCamp-Chengdu/HOP
Length of output: 96
🏁 Script executed:
# Search for signOut call sites (without tsx type filter)
rg "signOut\(" --type ts -B 2 -A 2Repository: FreeCodeCamp-Chengdu/HOP
Length of output: 1326
🏁 Script executed:
# Also try broader search for .tsx files
rg "signOut\(" -B 2 -A 2 | head -50Repository: FreeCodeCamp-Chengdu/HOP
Length of output: 1326
Harden sign-out against IndexedDB cleanup failures.
signOut() is async and awaits clear() at line 67, but all call sites invoke it without error handling (pages/user/[id].tsx and components/User/UserBar.tsx click handlers). When IndexedDB is unavailable or blocked (private mode, storage restrictions), clear() rejects, causing an unhandled promise rejection and preventing line 69's reload from executing.
🛡️ Suggested resilient implementation
async signOut(reload = false) {
setCookie('token', '', { path: '/', expires: new Date() });
setCookie('JWT', '', { path: '/', expires: new Date() });
this.user = undefined;
await sleep();
- await clear();
+ if (!isServer()) {
+ try {
+ await clear();
+ } catch {
+ // Keep sign-out successful even if client storage cleanup fails
+ }
+ }
- if (reload) location.reload();
+ if (reload && !isServer()) location.reload();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async signOut(reload = false) { | |
| setCookie('token', '', { path: '/', expires: new Date() }); | |
| setCookie('JWT', '', { path: '/', expires: new Date() }); | |
| this.user = undefined; | |
| await sleep(); | |
| await clear(); | |
| if (reload) location.reload(); | |
| async signOut(reload = false) { | |
| setCookie('token', '', { path: '/', expires: new Date() }); | |
| setCookie('JWT', '', { path: '/', expires: new Date() }); | |
| this.user = undefined; | |
| await sleep(); | |
| if (!isServer()) { | |
| try { | |
| await clear(); | |
| } catch { | |
| // Keep sign-out successful even if client storage cleanup fails | |
| } | |
| } | |
| if (reload && !isServer()) location.reload(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@models/User/Session.ts` around lines 61 - 69, The signOut function can throw
if clear() (IndexedDB cleanup) rejects, causing unhandled rejections and
preventing the subsequent reload; update async signOut(reload = false) to catch
and swallow errors from clear() (e.g., try { await clear(); } catch (err) { /*
log/warn but continue */ }) so failures in clear() do not block this.user reset
or the optional location.reload(), and add a minimal debug log inside the catch
to aid diagnosis; ensure callers in pages/user/[id].tsx and
components/User/UserBar.tsx can continue to call signOut() without surrounding
error handling.
| if (tips) alert(tips); | ||
| }); | ||
|
|
||
| sessionStore.getProfile().catch(console.debug); |
There was a problem hiding this comment.
Replace console.debug with an allowed logger (or remove it).
Line 50 violates no-console (only warn, error, info are allowed in this config).
✅ Minimal lint-safe patch
- sessionStore.getProfile().catch(console.debug);
+ sessionStore.getProfile().catch(console.info);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sessionStore.getProfile().catch(console.debug); | |
| sessionStore.getProfile().catch(console.info); |
🧰 Tools
🪛 ESLint
[error] 50-50: Unexpected console statement. Only these console methods are allowed: warn, error, info.
(no-console)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/_app.tsx` at line 50, The call
sessionStore.getProfile().catch(console.debug) uses console.debug which violates
the no-console rule; replace it with an allowed logger or remove the catch
handler. Update the catch to use an approved method (e.g.,
sessionStore.getProfile().catch(err => logger.warn('getProfile failed', err)) or
.catch(err => console.info(err)) if a shared logger isn't available), or remove
the .catch entirely if upstream handles errors; locate the occurrence by
searching for sessionStore.getProfile() in the file and change the catch
argument from console.debug to an allowed logger method (warn/info/error) or a
no-op.
| margin-bottom: 1.5rem; | ||
| border: none; | ||
|
|
||
| :global(.nav-link) { |
There was a problem hiding this comment.
Stylelint is currently failing on CSS Modules :global selectors.
Line 33 and Line 47 trigger selector-pseudo-class-no-unknown, so this file won’t pass lint as configured.
💡 Local fix option in this file
.custom-tabs {
gap: 0.5rem;
margin-bottom: 1.5rem;
border: none;
+ /* stylelint-disable-next-line selector-pseudo-class-no-unknown */
:global(.nav-link) {
@@
- &:global(.active) {
+ /* stylelint-disable-next-line selector-pseudo-class-no-unknown */
+ &:global(.active) {
background: `#2563eb`;
color: white;
}
}
}Also applies to: 47-47
🧰 Tools
🪛 Stylelint (17.9.0)
[error] 33-33: Unknown pseudo-class selector ":global" (selector-pseudo-class-no-unknown)
(selector-pseudo-class-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/user/`[id].module.less at line 33, Stylelint flags the CSS Modules
:global(...) pseudo-class as unknown for selectors like :global(.nav-link) (and
the other :global(...) at the later selector). Add an inline stylelint disable
comment immediately above each offending selector, e.g. /*
stylelint-disable-next-line selector-pseudo-class-no-unknown */ so stylelint
ignores the :global pseudo-class for those lines, or alternatively update
project stylelint config to allow the :global pseudo-class if you prefer a
global fix; target the specific selectors :global(.nav-link) and the other
:global(...) occurrences when applying the change.
| }, | ||
| }; | ||
| } | ||
| return result; |
There was a problem hiding this comment.
Fix lint rule before return result.
Line 24 is flagged by @stylistic/padding-line-between-statements; add a blank line before this return.
🧰 Tools
🪛 ESLint
[error] 24-24: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/user/me.tsx` at line 24, Add a blank line before the existing "return
result;" statement to satisfy the `@stylistic/padding-line-between-statements`
rule; locate the return of the variable "result" in the function or component
that contains "return result;" and insert one empty line immediately above it.
TechQuery
added
feature
TechQuery
changed the title
2 participants
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores